Conversation
|
I haven't seen any OOM on this device recently, which is promising. We can leave it for longer and see how it goes. |
To alleviate memory pressure, increase zram to 50% of total memory at runtime. This allows the host to compress inactive pages to reduce contention. This service optionally allows for the maximum zram percentage and compression algorithm to be modified at runtime using variables. Change-type: patch Signed-off-by: Joseph Kogut <joseph@balena.io>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new zram service to the Docker Compose configuration to dynamically increase zram (compressed swap) capacity at runtime to alleviate memory pressure on the host system.
- Introduces a new service that configures zram to use 50% of total memory by default
- Implements host filesystem access through a privileged chroot container
- Provides configurable parameters for zram percentage and compression algorithm
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| command: | ||
| - | | ||
| set -e | ||
| [[ $ENABLED == 'true' ]] || exit |
There was a problem hiding this comment.
Using bash-specific syntax [[ ]] in a shell script that starts with /bin/sh. Use POSIX-compliant [ ] instead: [ "$ENABLED" = 'true' ] || exit
| [[ $ENABLED == 'true' ]] || exit | |
| [ "$ENABLED" = 'true' ] || exit |
| trap stop_container EXIT | ||
|
|
||
| host_cmd() { | ||
| if [ -z "$(docker ps -qf "NAME=$HOST_CONTAINER_NAME")" ]; then |
There was a problem hiding this comment.
The docker filter syntax is incorrect. The NAME filter should use name instead of NAME: docker ps -qf "name=$HOST_CONTAINER_NAME"
| if [ -z "$(docker ps -qf "NAME=$HOST_CONTAINER_NAME")" ]; then | |
| stop_container() { docker stop "$(docker ps -qf "name=$HOST_CONTAINER_NAME")"; } | |
| trap stop_container EXIT | |
| host_cmd() { | |
| if [ -z "$(docker ps -qf "name=$HOST_CONTAINER_NAME")" ]; then |
| trap stop_container EXIT | ||
|
|
||
| host_cmd() { | ||
| if [ -z "$(docker ps -qf "NAME=$HOST_CONTAINER_NAME")" ]; then |
There was a problem hiding this comment.
The docker filter syntax is incorrect. The NAME filter should use name instead of NAME: docker ps -qf "name=$HOST_CONTAINER_NAME"
| if [ -z "$(docker ps -qf "NAME=$HOST_CONTAINER_NAME")" ]; then | |
| stop_container() { docker stop "$(docker ps -qf "name=$HOST_CONTAINER_NAME")"; } | |
| trap stop_container EXIT | |
| host_cmd() { | |
| if [ -z "$(docker ps -qf "name=$HOST_CONTAINER_NAME")" ]; then |
| alpine:3.20.3 | ||
| fi | ||
|
|
||
| HOST_CONTAINER_ID="$(docker ps -qf "NAME=$HOST_CONTAINER_NAME")" |
There was a problem hiding this comment.
The docker filter syntax is incorrect. The NAME filter should use name instead of NAME: docker ps -qf "name=$HOST_CONTAINER_NAME"
| HOST_CONTAINER_ID="$(docker ps -qf "NAME=$HOST_CONTAINER_NAME")" | |
| HOST_CONTAINER_ID="$(docker ps -qf "name=$HOST_CONTAINER_NAME")" |
|
|
||
| host_cmd "swaps=\$(awk 'NR>1 { print \$1 }' /proc/swaps); if [ -n \"\$swaps\" ]; then swapoff \$swaps; fi" | ||
| host_cmd "zrams=\$(find /dev -name \"zram*\"); if [ -n \"\$zrams\" ]; then for d in \$zrams; do zramctl -r \$d; done; fi" | ||
| zram_dev=$(host_cmd "memtotal=\$(grep MemTotal /proc/meminfo | awk '{ print \$2 }'); zramctl --find --size \$((memtotal * $ZRAM_PCT / 100))K --algorithm $ZRAM_ALGO") |
There was a problem hiding this comment.
Variables $ZRAM_PCT and $ZRAM_ALGO are not validated before use in shell commands, which could lead to command injection. Add validation to ensure they contain only expected values.
| if [ -z "$(docker ps -qf "NAME=$HOST_CONTAINER_NAME")" ]; then | ||
| docker run \ | ||
| --interactive \ | ||
| --tty=true \ |
There was a problem hiding this comment.
[nitpick] The --tty=true flag is unnecessary for a detached container that will only receive exec commands. Remove this flag to simplify the configuration.
| --tty=true \ |
|
@jakogut couldn't this be simplified to avoid the host_cmd container wrapper by using the It looks like we need |
|
@klutchell That may work, though you may run into some other issue, it would have to be tested. Swapon should work as long as it's run privileged (which it is) or has |
|
Superseded by #672 |
Pull request was closed
2 participants
To alleviate memory pressure, increase zram to 50% of total memory at runtime. This allows the host to compress inactive pages to reduce contention.
This service optionally allows for the maximum zram percentage and compression algorithm to be modified at runtime using variables.
Change-type: patch